Skip to content

security: pin TLS 1.2 floor in RestClient and EventListener#162

Merged
petrsnd merged 9 commits into
OneIdentity:mainfrom
petrsnd:security/review-20260522-java
May 27, 2026
Merged

security: pin TLS 1.2 floor in RestClient and EventListener#162
petrsnd merged 9 commits into
OneIdentity:mainfrom
petrsnd:security/review-20260522-java

Conversation

@petrsnd
Copy link
Copy Markdown
Member

@petrsnd petrsnd commented May 26, 2026

Summary

Pin SSLContext.getInstance("TLS") to SSLContext.getInstance("TLSv1.2") in both REST and SignalR transports. The generic "TLS" alias can resolve to TLS 1.0/1.1 on misconfigured JVMs; pinning the version at the SDK layer guarantees a TLS 1.2+ handshake regardless of JVM configuration. TLS 1.3 is still negotiated when supported by both peers.

Changes

  • TLS pinned to TLSv1.2 floor in RestClient and SafeguardEventListener — eliminates fallback to TLS 1.0/1.1
  • Unit tests validating the TLS protocol constant and SSLContext instantiation
  • JUnit 4 + maven-surefire-plugin added for test infrastructure
  • README section documenting ignoreSsl flag behavior and production guidance
  • Javadoc on RestClient and SafeguardEventListener constructors explaining ignoreSsl semantics
  • Version bump to 8.2.1-SNAPSHOT

Verification

  • mvn verify passes (4 unit tests, 0 failures; SpotBugs clean)
  • Full integration regression against live appliance: 59 passed, 0 failed

@petrsnd petrsnd requested a review from a team as a code owner May 26, 2026 21:46
@petrsnd
Copy link
Copy Markdown
Member Author

petrsnd commented May 26, 2026

Partial live verification against 192.168.117.15 admin smoke 2026-05-26. Manual Java SDK read-only probe passed (connect, GET Me, GET Users?limit=200, GET Status). Oversized response cap could not be safely triggered with available read-only appliance endpoints; cap remains unit-verified only. Full log: .security-review-impl-logs/live-sweep/java-live.log

@petrsnd
Copy link
Copy Markdown
Member Author

petrsnd commented May 27, 2026

Full live appliance sweep re-run (mutation allowed) completed against 192.168.117.15.

Results:

  • SafeguardDotNet (security/review-20260522-dotnet): 15 suites, 71 passed / 0 failed / 2 skipped. SpsIntegration excluded because no SPS appliance was in the lease. Cleanup audit: no SgDnTest objects remained.
  • PySafeguard (security/review-20260522-py): after installing the optional SignalR extra required by event tests, full pytest passed: 453 passed / 0 failed / 0 skipped. Cleanup audit found one leaked PySg_ event-test user; it was deleted and re-audit showed 0 remaining.
  • safeguard.js (security/review-20260522-js): integration suite passed: 11 files, 55 passed / 0 failed / 0 skipped. Cleanup audit: no SgJs_ objects remained.
  • safeguard-bash (security/review-20260522-bash): full suite executed with SAFEGUARD_ALLOW_LOCALHOST=1 after the stock runner PKCE preflight failed against the private appliance address. Result: 14 suites, 323 passed / 10 failed / 0 skipped. Failures are confined to A2A and A2A Access Request Broker retrieval/broker negative-path checks. Cleanup audit: no SgBashTest objects remained.
  • SafeguardJava (security/review-20260522-java): PowerShell integration runner passed: 9 suites, 59 passed / 0 failed / 0 skipped; SpsIntegration excluded because no SPS appliance was in the lease. FP-004 cap regression unit test also passed: 6 passed / 0 failed / 0 skipped. Cleanup audit: no SgJTest objects remained.

Lease released in SECURITY-REVIEW.md. Follow-up needed: investigate safeguard-bash A2A failures and the PySafeguard event-test cleanup leak.

petrsnd added 7 commits May 27, 2026 16:43
…-SafeguardJava-001)

Replaces the generic SSLContext.getInstance(TLS) call with an explicit
TLSv1.2 protocol pin in both the REST transport (RestClient.getSSLContext)
and the SignalR/WebSocket transport (SafeguardEventListener.ConfigureHttpClientBuilder).

The TLS alias can resolve to TLS 1.0/1.1 on JVMs whose
jdk.tls.disabledAlgorithms has been narrowed; pinning the version at the
SDK layer makes the minimum handshake version a property of the SDK itself,
independent of caller JVM configuration. TLS 1.3, where supported by both
peers, is still negotiated normally by the underlying SSLContext.

Both classes now expose a package-private TLS_PROTOCOL constant so the
pinned version has a single auditable source of truth.

Tests:
- RestClientSSLContextTest: verifies TLS_PROTOCOL constant + actual
  SSLContext.getProtocol() value via reflection on getSSLContext.
- SafeguardEventListenerSSLContextTest: mirror test for the event listener
  transport.

Test infrastructure: pom.xml now declares junit 4.13.2 + okhttp3
mockwebserver 4.12.0 (test scope) and maven-surefire-plugin 3.2.5;
previously there was no src/test/java suite at all.

Resolves F-SafeguardJava-003 (W2).
…uardJava-002)

Per cross-cutting decision D-001, the ignoreSsl feature is kept as a
legitimate convenience for development against self-signed appliances,
and the SDK does not emit a runtime warning. This commit closes the
documentation gap:

- README.md: new section 'TLS Certificate Verification and the
  ignoreSsl Flag'. Documents what ignoreSsl actually toggles (X.509 chain
  validation + a NoopHostnameVerifier, *not* the TLS version), a
  three-row table of supported configurations, and the recommended
  production paths (JVM truststore import, or a HostnameVerifier callback
  with ignoreSsl=false).
- RestClient.java: Javadoc on the (String, String, char[], boolean,
  HostnameVerifier) constructor describing the security implications of
  ignoreSsl and pointing to the TLS_PROTOCOL constant.
- SafeguardEventListener.java: equivalent Javadoc on the (String,
  char[], boolean, HostnameVerifier) constructor.

No code or public API changes.

Resolves F-SafeguardJava-001, F-SafeguardJava-002, F-SafeguardJava-004 (W3).
Add BoundedResponseReader that enforces a 10 MB default cap on in-memory HTTP response bodies. Pre-read check rejects oversized Content-Length headers; streaming counter trips on chunked overflow. Wired into Utils.getResponse and PkceAuthenticator.rstsFormPost. Explicit streaming download paths (StreamResponse, StreamingRequest) are unaffected — callers there manage their own sinks.
Audit outcome: Java event listener delegates reconnect to caller via SafeguardEventListenerDisconnectedException; no native tight-loop reconnect exists to harden. Distinct-but-valid design. A jittered exponential backoff helper is deferred to Phase 2.
…sion

- Remove BoundedResponseReader and ResponseTooLargeException (not needed;
  Safeguard appliances are closed hardened systems)
- Revert Utils.getResponse and PkceAuthenticator to use EntityUtils
- Remove mockwebserver test dependency and response size cap test
- Remove handleDisconnect Phase-2 reconnect comment
- Strip internal finding IDs from test Javadoc
- Version 8.2.1-SNAPSHOT (patch bump, not minor)
@petrsnd petrsnd force-pushed the security/review-20260522-java branch from 451a40c to d8835a5 Compare May 27, 2026 23:13
@petrsnd petrsnd changed the title security: SafeguardJava 8.3.0 — coordinated security review fixes security: pin TLS 1.2 floor in RestClient and EventListener May 27, 2026
petrsnd added 2 commits May 27, 2026 17:38
SignalR 8.0.27 requires Java 9+ class format. Use Class.forName with
UnsupportedClassVersionError catch to skip the test on Java 8 runtimes
rather than failing the entire build.
The SignalR client dependency requires Java 9+ at runtime. All other
SDK features remain compatible with Java 8.
@petrsnd petrsnd merged commit 948d4c1 into OneIdentity:main May 27, 2026
4 checks passed
@petrsnd petrsnd deleted the security/review-20260522-java branch May 27, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant